Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Expose size_hint() for TokenStream's iterator #99703

Merged
merged 1 commit into from
Jul 26, 2022
Merged

Expose size_hint() for TokenStream's iterator #99703

merged 1 commit into from
Jul 26, 2022

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Jul 25, 2022

The iterator for proc_macro::TokenStream is a wrapper around a Vec iterator:

rust/library/proc_macro/src/lib.rs

Lines 363 to 371 in babff22

pub struct IntoIter(
std::vec::IntoIter<
bridge::TokenTree<
bridge::client::TokenStream,
bridge::client::Span,
bridge::client::Symbol,
>,
>,
);

so it can cheaply provide a perfectly precise size hint, with just a pointer subtraction:

rust/library/alloc/src/vec/into_iter.rs

Lines 170 to 177 in babff22

fn size_hint(&self) -> (usize, Option<usize>) {
let exact = if mem::size_of::<T>() == 0 {
self.end.addr().wrapping_sub(self.ptr.addr())
} else {
unsafe { self.end.sub_ptr(self.ptr) }
};
(exact, Some(exact))
}

I need the size hint in syn (https://github.com/dtolnay/syn/blob/1.0.98/src/buffer.rs) to reduce allocations when converting TokenStream into syn's internal TokenBuffer representation.

Aside from size_hint, the other non-default methods in std::vec::IntoIter's Iterator impl are advance_by, count, and __iterator_get_unchecked. I've included count in this PR since it is trivial. I did not include __iterator_get_unchecked because it is spoopy and I did not feel like dealing with that. Lastly, I did not include advance_by because that requires feature(iter_advance_by) (#77404) and I noticed this comment at the top of libproc_macro:

rust/library/proc_macro/src/lib.rs

Lines 20 to 22 in babff22

// This library is copied into rust-analyzer to allow loading rustc compiled proc macros.
// Please avoid unstable features where possible to minimize the amount of changes necessary
// to make it compile with rust-analyzer on stable.

@rustbot
Copy link
Collaborator

rustbot commented Jul 25, 2022

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 25, 2022
@dtolnay dtolnay added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jul 25, 2022
@dtolnay dtolnay mentioned this pull request Jul 25, 2022
Merged
@petrochenkov
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 25, 2022

📌 Commit 63e74ab has been approved by petrochenkov

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 25, 2022
@mystor
Copy link
Contributor

mystor commented Jul 25, 2022

I'm definitely considering changing the iterator to not be a thin wrapper around Vec's IntoIter in the future, but providing size_hint() should be something we're generally able to do. Main worry I have going forward is if I end up moving tokens into the client more, and we need to split a TokenTree in two due to it being a Literal(-1) or similar, but that's probably not going to be an issue.

@JohnTitor JohnTitor mentioned this pull request Jul 25, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2022
@bors
Auto merge of rust-lang#99735 - JohnTitor:rollup-d93jyr2, r=JohnTitor
a867059 
Rollup of 9 pull requests

Successful merges:

 - rust-lang#92390 (Constify a few `(Partial)Ord` impls)
 - rust-lang#97077 (Simplify some code that depend on Deref)
 - rust-lang#98710 (correct the output of a `capacity` method example)
 - rust-lang#99084 (clarify how write_bytes can lead to UB due to invalid values)
 - rust-lang#99178 (Lighten up const_prop_lint, reusing const_prop)
 - rust-lang#99673 (don't ICE on invalid dyn calls)
 - rust-lang#99703 (Expose size_hint() for TokenStream's iterator)
 - rust-lang#99709 (`Inherited` always has `TypeckResults` available)
 - rust-lang#99713 (Fix sidebar background)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5bbdf65 into rust-lang:master Jul 26, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 26, 2022
@dtolnay dtolnay deleted the tokenstreamsizehint branch October 24, 2023 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@petrochenkov petrochenkov

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.64.0
Development

Successfully merging this pull request may close these issues.
6 participants
@dtolnay @rustbot @rust-highfive @petrochenkov @bors @mystor